Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

disable jobserver on unix, if file descriptors are negative #68

Merged
merged 1 commit into from
Feb 6, 2024
Merged

disable jobserver on unix, if file descriptors are negative #68

merged 1 commit into from
Feb 6, 2024
+16 −0

Conversation

belovdv
Copy link
Contributor

@belovdv belovdv commented Feb 1, 2024 •
edited
Loading

According to the GNU make manual:

If either or both of these file descriptors are negative, it means the jobserver is disabled for this process.

This pr makes so.

From rust-lang/rust#120515.

weihanglo
weihanglo reviewed Feb 1, 2024
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems good to me :)

src/unix.rs Outdated Show resolved Hide resolved
@belovdv belovdv force-pushed the consider-negative-file-descriptors branch from eabe348 to 1ea961f Compare February 1, 2024 19:06
ojeda
ojeda reviewed Feb 1, 2024
View reviewed changes
src/error.rs Outdated Show resolved Hide resolved
src/unix.rs Show resolved Hide resolved
@ojeda
Copy link

ojeda commented Feb 1, 2024

Thanks for solving this!

@ojeda ojeda mentioned this pull request Feb 1, 2024
Open
47 tasks
@belovdv belovdv force-pushed the consider-negative-file-descriptors branch 2 times, most recently from be32a7d to d4107e4 Compare February 1, 2024 21:09
ojeda
ojeda reviewed Feb 3, 2024
View reviewed changes
src/unix.rs Outdated Show resolved Hide resolved
ojeda
ojeda reviewed Feb 3, 2024
View reviewed changes
src/error.rs Outdated Show resolved Hide resolved
@belovdv belovdv force-pushed the consider-negative-file-descriptors branch from d4107e4 to 498af46 Compare February 4, 2024 10:58
@belovdv
Copy link
Contributor Author

belovdv commented Feb 4, 2024

With separated NegativeFd error kind, which has extensive comment I'd expect to see more explanations of what CannotParse error means and why doesn't it follow manual:

If your tool does not recognize the format of the --jobserver-auth string, it should assume the jobserver is using a different style and it cannot connect.

Not a part of this PR, though

@belovdv belovdv force-pushed the consider-negative-file-descriptors branch from 498af46 to b89d6fd Compare February 4, 2024 11:11
petrochenkov
petrochenkov reviewed Feb 5, 2024
View reviewed changes
src/error.rs Outdated Show resolved Hide resolved
src/error.rs Outdated Show resolved Hide resolved
src/error.rs Outdated Show resolved Hide resolved
src/error.rs Outdated Show resolved Hide resolved
@belovdv belovdv force-pushed the consider-negative-file-descriptors branch from b89d6fd to d8dc8d8 Compare February 6, 2024 19:26
@petrochenkov petrochenkov merged commit 7f6aea0 into rust-lang:main Feb 6, 2024
14 checks passed
@weihanglo
Copy link
Member

@petrochenkov do we plan to have a release for this and #67?

@petrochenkov
Copy link
Contributor

@weihanglo
Anything else non-controversial needs to go into the release? #57 maybe?

@weihanglo
Copy link
Member

@petrochenkov
Added comments to #57. I am fine with either way :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ojeda ojeda ojeda left review comments

@petrochenkov petrochenkov petrochenkov left review comments

@weihanglo weihanglo weihanglo left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@belovdv @ojeda @weihanglo @petrochenkov